[Command] Addition of a simple command `make_variant` #35

DEKHTIARJonathan · 2025-04-15T17:40:34Z

This PR adds a command variantlib make_variant to manually add variant information "post build" to a wheel.

You can see it as "repackaging a wheel as a variant after a build".

This will be very useful to allow everybody to quickly transform any wheel into a variant without having to modify absolutely anything else.

variantlib make_variant \
    -f pep_xxx_wheel_variants-1.0.0-py3-none-any.whl \
    -o . 
    -p fictional_tech::quantum::drive \
    -p fictional_hw::platform::TARS

After discussing with @warsawnv we thought that this was a critical feature to support initially (as build backends gradually rollout support) and for testing.

This PR also include a "optional extra" called user - Just because this command adds a feature on wheel which will not be useful in most cases (like for pip and I'm not sure how if pip and other would like very much having to bring wheel), we can change that part later if we think it's not a big deal to add wheel as a core dependency.

This PR uses #33

…o it.

variantlib/commands/make_variant.py

mgorny · 2025-04-15T18:13:31Z

variantlib/commands/make_variant.py

+
+    with tempfile.TemporaryDirectory() as _tmpdir:
+        tempdir = pathlib.Path(_tmpdir)
+        wheel_unpack(input_filepath, tempdir)


I don't think we really need to unpack it. Unfortunately, it looks like zipfile lacks proper API to update files in place or copy compressed members (python/cpython#125718), but at least we could avoid writing files to disk and potentially altering their metadata in the process.

I.e. basically open both archives, iterate over members: if not metadata, copy between archives as-is; if metadata, read, alter and write. It will probably even be simpler than the current logic, which seems to make unnecessary changes to the archive (probably because it was adapted from the command altering tags, which we don't do here).

I don't disagree though it's a little complicated for a first iteration - feel free to experiment and optimize ;)

variantlib/commands/make_variant.py

Co-authored-by: Michał Górny <[email protected]>

DEKHTIARJonathan · 2025-04-15T19:34:17Z

Good enough for a V1 of the command - merging so that a few people can start playing with it.
@mgorny feel free to adjust & optimize the command further :)

warsawnv

Thanks for writing this! I think this may end up being a very common way to create variant wheels.

Comments given; I hope I'm not being too nitpicky!

warsawnv · 2025-04-15T22:11:59Z

variantlib/commands/make_variant.py

+
+    # Input Validation
+    variant_hash_pattern = rf"^[a-fA-F0-9]{{{VARIANT_HASH_LEN}}}$"
+    if not re.match(variant_hash_pattern, variant_hash):


If wheel_variant_pack() is an internal API, then you control the variant_hash and this could probably be an assertion.

warsawnv · 2025-04-15T22:14:46Z

variantlib/commands/make_variant.py

+        raise whl_pck.WheelError(
+            f"Multiple .dist-info directories found in {directory}"
+        )
+    if not dist_info_dirs:


Since you're checking the length of a list, and you're explicitly calling len() above, it's probably best to also do so here, e.g. if len(dist_info_dirs) == 0:

warsawnv · 2025-04-15T22:16:06Z

variantlib/commands/make_variant.py

+        tags: list[str] = info.get_all("Tag", [])
+        existing_build_number = info.get("Build")
+
+        if not tags:


Another place where it's generally better to use if len(tags) == 0:

I generally prefer to be as explicit about the truthiness test as possible. E.g. if you know you're testing the length of a sequence, use len. If you are testing whether something is None or not, use if foo is None. Only when the truthiness or falseiness can be any number of things (e.g. None, the empty string, or a zero length sequence) do I typically use a generic if foo or if not foo. YMMV!

warsawnv · 2025-04-15T22:17:13Z

variantlib/commands/make_variant.py

+        if not tags:
+            raise whl_pck.WheelError(
+                f"No tags present in {dist_info_dir}/WHEEL; cannot determine target "
+                f"wheel filename"


I'm kind of surprised you didn't get a linter warning here, since the line 78 string doesn't need to be an f-string.

warsawnv · 2025-04-15T22:18:30Z

variantlib/commands/make_variant.py

+        del info["Build"]
+        if build_number:
+            info["Build"] = build_number
+            name_version += "-" + build_number


This one could be an f-string: name_version += f"-{build_number}"

warsawnv · 2025-04-15T22:19:39Z

pyproject.toml

 ]

 [project.optional-dependencies]
+user = [


What do you think about using cli (or commands) for the optional instead?

warsawnv · 2025-04-15T22:21:26Z

variantlib/commands/make_variant.py

+    Minimal changes tried to be applied to make it work with the Variant Hash.
+
+    :param directory: The unpacked wheel directory
+    :param dest_dir: Destination directory (defaults to the current directory)


Does it? Or is it required?

warsawnv · 2025-04-15T22:28:42Z

variantlib/commands/make_variant.py

+        if not (metadata_f := distinfo_dir / "METADATA").exists():
+            raise FileNotFoundError(metadata_f)
+
+        with metadata_f.open(mode="r+") as file:


Do you think it would be better to write a new file and move it into place atomically? E.g. copy lines from METADATA into METADATA.new, then os.rename() the latter to the former. The question I have is, what if this loop errors out or is interrupted in the middle of it? You can probably also avoid the seek() back to the beginning and the truncate().

You probably still need a try/except that deletes the .new file and cleans up after itself, reraising the original exception, but at least then you can still recover back to the original state if something unexpected happens.

warsawnv · 2025-04-15T22:31:37Z

variantlib/constants.py

+METADATA_VARIANT_HASH_HEADER = "Variant-hash"
+METADATA_VARIANT_PROPERTY_HEADER = "Variant"
+
+WHEEL_NAME_VALIDATION_REGEX = re.compile(


A multi-line regex string with comments would help with readability here.

In general, I agree here but the regex is copied (and slightly modified) from pip (and we already have it repeated in too many places).

DEKHTIARJonathan · 2025-04-16T03:52:03Z

Thanks a lot @warsawnv for your comments. Many of the comments you made are actually directly targeting the original implementation from wheel.cli.pack. I'm gonna check tomorrow what is the original code (which I don't want to modify) and which one my code (and I can improve).

I believe in the future some of that code may go inside wheel itself. Let's see.

In any case thanks for the comments, I'll check what I can improve and I'll open a new PR with these changes.

* [Command] `generate-index-json` updated. According to #35 * Review Fixes

DEKHTIARJonathan requested review from mgorny and warsawnv April 15, 2025 17:40

DEKHTIARJonathan force-pushed the commands branch 2 times, most recently from 02f7a23 to 40fe15e Compare April 15, 2025 17:47

Basic Command to "repackage" a normal wheel and hack variant info int…

b93b2f5

…o it.

DEKHTIARJonathan force-pushed the commands branch from 40fe15e to b93b2f5 Compare April 15, 2025 17:47

mgorny requested changes Apr 15, 2025

View reviewed changes

DEKHTIARJonathan and others added 5 commits April 15, 2025 15:13

Update variantlib/commands/make_variant.py

3852ce6

Co-authored-by: Michał Górny <[email protected]>

Update variantlib/commands/make_variant.py

159a22f

Co-authored-by: Michał Górny <[email protected]>

Update variantlib/commands/make_variant.py

39ce5e5

Co-authored-by: Michał Górny <[email protected]>

Update variantlib/commands/make_variant.py

327f451

Co-authored-by: Michał Górny <[email protected]>

Fixes

50693bf

DEKHTIARJonathan merged commit 4d0c025 into main Apr 15, 2025
16 checks passed

DEKHTIARJonathan deleted the commands branch April 15, 2025 19:34

warsawnv reviewed Apr 15, 2025

View reviewed changes

DEKHTIARJonathan added a commit that referenced this pull request May 2, 2025

Changing constants with new structure from #35

dd732a8

DEKHTIARJonathan added a commit that referenced this pull request May 2, 2025

Changing constants with new structure from #35 - Adding Req Validator

9727544

DEKHTIARJonathan added a commit that referenced this pull request May 2, 2025

[Command] generate-index-json updated. According to #35

1bf58c6

DEKHTIARJonathan added a commit that referenced this pull request May 2, 2025

[Command] generate-index-json updated. According to #35

bdf1ca4

DEKHTIARJonathan added a commit that referenced this pull request May 2, 2025

[Command] generate-index-json updated. According to #35

121283c

DEKHTIARJonathan added a commit that referenced this pull request May 2, 2025

[Command] generate-index-json updated. According to #35

bdfc483

DEKHTIARJonathan added a commit that referenced this pull request May 2, 2025

[Command] generate-index-json updated. According to #35

49f32d7

DEKHTIARJonathan added a commit that referenced this pull request May 2, 2025

[Command] generate-index-json updated. According to #35 (#72)

2990120

* [Command] `generate-index-json` updated. According to #35 * Review Fixes

Uh oh!

[Command] Addition of a simple command make_variant #35

[Command] Addition of a simple command make_variant #35

Conversation

DEKHTIARJonathan commented Apr 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DEKHTIARJonathan commented Apr 15, 2025

Uh oh!

Uh oh!

warsawnv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DEKHTIARJonathan commented Apr 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[Command] Addition of a simple command `make_variant` #35

[Command] Addition of a simple command `make_variant` #35